Please take a moment and address the merge conflicts of your pull request. Thanks!

I can see a scenario where we end up with one ApplyTask instance effectively only utilising one thread for a 512x512 region of the edit, causing the whole edit to wait for it when everything else is finished. I don't know if there's a particular way around that, but in a complex operation this could be quite a perceived performance hit

Yes, that's possible in theory, and the heuristic when to split is rather simple currently. In practice, it would mean the thread pool is already busy and there are even more tasks waiting. We could e.g. force splitting on 512x512 regions, or check ForkJoinTask.getSurplusQueuedTaskCount() > Settings.settings().QUEUE.PARALLEL_THREADS * f(this.shift) (where f is some "damping" function) to make that case even less likely.

I also though about re-checking the heuristics during processing, but that complicates things too much I think.

I can see a scenario where we end up with one ApplyTask instance effectively only utilising one thread for a 512x512 region of the edit, causing the whole edit to wait for it when everything else is finished. I don't know if there's a particular way around that, but in a complex operation this could be quite a perceived performance hit

Yes, that's possible in theory, and the heuristic when to split is rather simple currently. In practice, it would mean the thread pool is already busy and there are even more tasks waiting. We could e.g. force splitting on 512x512 regions, or check ForkJoinTask.getSurplusQueuedTaskCount() > Settings.settings().QUEUE.PARALLEL_THREADS * f(this.shift) (where f is some "damping" function) to make that case even less likely.

I also though about re-checking the heuristics during processing, but that complicates things too much I think.

If we are processing a larger region by smaller region at a time in the first place, rather than the for x; for z I think it would be much more reasonable to check if we want to parallelise further at the end of the completion of each subregion, and can therefore submit the rest. E.g. create a queue at the start of a region's processing and just submit the remaining as/when needed

If we are processing a larger region by smaller region at a time in the first place, rather than the for x; for z I think it would be much more reasonable to check if we want to parallelise further at the end of the completion of each subregion, and can therefore submit the rest. E.g. create a queue at the start of a region's processing and just submit the remaining as/when needed

Hm I guess processing chunks using a hilbert curve might be useful there. Otherwise, just always splitting and then unforking is basically that, and the overhead of creating that many tasks eagerly is most likely not a problem.

I reworked the heuristic to directly process a region a bit:

• With larger shifts, we fork more aggressive (e.g. on the highest level where we split into 512x512 (blocks) regions, we need to split off 32 tasks (32 region files) that aren't taken by other threads yet before processing a full region directly)
• Even 32x32 (blocks) regions are split further unless there are more than 3 tasks in the queue of the current thread

With this design, I think it is very unlikely that we process overly large regions that take more time than what the pool can process currently. We can explore more dynamic approaches in future, but I'd like to start with a mainly simple solution.

I reworked the heuristic to directly process a region a bit:

• With larger shifts, we fork more aggressive (e.g. on the highest level where we split into 512x512 (blocks) regions, we need to split off 32 tasks (32 region files) that aren't taken by other threads yet before processing a full region directly)
• Even 32x32 (blocks) regions are split further unless there are more than 3 tasks in the queue of the current thread

With this design, I think it is very unlikely that we process overly large regions that take more time than what the pool can process currently. We can explore more dynamic approaches in future, but I'd like to start with a mainly simple solution.

I suppose it's hard to make much judgement without some kinda of usage statistics from servers with a lot of players on. I don't know if we would want to effectively enforce single-threaded operation per edit. For now it's probably fine I suppose, but worth seeing if we can keep an eye on this in bstats somehow?

I suppose it's hard to make much judgement without some kinda of usage statistics from servers with a lot of players on. I don't know if we would want to effectively enforce single-threaded operation per edit. For now it's probably fine I suppose, but worth seeing if we can keep an eye on this in bstats somehow?

It's generally difficult to tell for me how people are using FAWE (which patterns, masks, brushes, typical edit sizes, etc) so I'd like to explore some kind of observability that can be shared (probably similar to /fawe debugpaste). But running into the scenario where an edit becomes single-threaded when multiple cores are available has an extremely low likelihood (previous long-running tasks would need to take up all but one thread and end all basically at the same time). I'll try to work on something more dynamic in the future, but that will most likely increase complexity and it might take me some weeks.

Yeah I think the groundwork here is probably worth a merge for now. As it is I don't know that large edits would be done very often, and I do also wonder if perhaps for small edits (<ncpu chunks) we should just throw it in single-thread anyway